Skip to content

Fix cluster-wide Kafka client bug#14857

Merged
ChrsMark merged 2 commits intoelastic:masterfrom
ChrsMark:fix_never_reopened_kafka_client
Nov 29, 2019
Merged

Fix cluster-wide Kafka client bug#14857
ChrsMark merged 2 commits intoelastic:masterfrom
ChrsMark:fix_never_reopened_kafka_client

Conversation

@ChrsMark
Copy link
Copy Markdown
Member

This PR fixes a bug introduced on #14822.

The cluster-wide client was never re-opened after the first call to Fetch method because all the other times the Connect method to broker was returning before the cluster-wide client initialisation, due to

if b.id != noID || !b.matchID {
.

cc: @jsoriano @mtojek

Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark requested a review from a team as a code owner November 29, 2019 11:01
@ChrsMark ChrsMark self-assigned this Nov 29, 2019
Copy link
Copy Markdown
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if it's possible to cover these changes with an integration test or sth. Anyway, approved!

Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Copy Markdown
Member Author

ChrsMark commented Nov 29, 2019

@mtojek thanks for reviewing! I checked about the test addition and I tuned a little bit the current one so as to Fetch data 3 times in order to verify that connect/disconnect are not broken after first Fetch.

@mtojek
Copy link
Copy Markdown
Contributor

mtojek commented Nov 29, 2019

Cool! Thanks for the heads up.

@ChrsMark ChrsMark merged commit 6c279eb into elastic:master Nov 29, 2019
@ChrsMark ChrsMark added the needs_backport PR is waiting to be backported to other branches. label Nov 29, 2019
ChrsMark added a commit to ChrsMark/beats that referenced this pull request Nov 29, 2019
@ChrsMark ChrsMark removed the needs_backport PR is waiting to be backported to other branches. label Nov 29, 2019
ChrsMark added a commit to ChrsMark/beats that referenced this pull request Nov 29, 2019
ChrsMark added a commit that referenced this pull request Nov 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug review Team:Integrations Label for the Integrations team v7.6.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants